Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vizia Update #88

Closed
wants to merge 10 commits into from
Closed

Vizia Update #88

wants to merge 10 commits into from

Conversation

geom3trik
Copy link
Contributor

No description provided.

@robbert-vdh
Copy link
Owner

Thanks for the PR! A couple things that didn't work properly last time I looked at this seem to work better now, so that's great!

When I merge this, I"ll end up cherry picking most of the relevant changes into my own vizia update branch I'd been working on. I'll need to redo the changes to the non-example plugins for copyright purposes (I want to keep full copyright over the GPL'd plugins so I can use bits and pieces in later projects if I want to).

Some things I noticed that still need work are:

  • The styling and handling for text entry in all of the widgets is still broken. I'll have to see if I can fix this on nih_plug_vizia's side or if these are regressions on Vizia.
  • A lot of positioning and sizing logic had to be juggled around. Is the current approach from this PR going to be the final behavior for Vizia 1.0, or are things still in flux?
  • Font handling broke between the old Vizia version and this one. That's a cosmic-text issue. The workaround of having to explicitly specify font weights instead of being able to use alternative family names like Noto Sans Thin is a bit of a pain but I don't see them fixing this bug anytime soon.
  • This PR still has lots of warnings and debugging code, but I have already fixed most of those warnings in my branch from a couple months ago so I'll just cherry pick the remaining changes there instead.

@geom3trik
Copy link
Contributor Author

Some things I noticed that still need work are:

  • The styling and handling for text entry in all of the widgets is still broken. I'll have to see if I can fix this on nih_plug_vizia's side or if these are regressions on Vizia.

I pushed a change to vizia and this PR that should fix this now.

  • A lot of positioning and sizing logic had to be juggled around. Is the current approach from this PR going to be the final behavior for Vizia 1.0, or are things still in flux?

I've mentioned it before somewhere but will repeat it here for clarity, the main difference is that the old morphorm had Auto min-size, which would stop the parent from being smaller than the children even if the width/height was fixed. I tried to replicate this in new morphorm recently but failed and had to revert. Therefore, the new version of morphorm does not support auto min-size and if you want that behaviour then the closest thing is to specify auto size on the parent instead. I'm not planning on changing this behaviour any time soon and not before the first release.

  • Font handling broke between the old Vizia version and this one. That's a cosmic-text issue. The workaround of having to explicitly specify font weights instead of being able to use alternative family names like Noto Sans Thin is a bit of a pain but I don't see them fixing this bug anytime soon.

It is unfortunate but I have tried to address this in the PR.

  • This PR still has lots of warnings and debugging code, but I have already fixed most of those warnings in my branch from a couple months ago so I'll just cherry pick the remaining changes there instead.

Okie dokey 👍

@cyypherus
Copy link

I'm curious, has the patched fork diverged from Vizia such that you don't plan to merge the changes? Is there a reason not to PR the changes & depend on the original repo?

@robbert-vdh
Copy link
Owner

There are no diverging changes. Every patch has been upstreamed already a long time ago. The version of Vizia nih_plug_vizia uses right now is just an old version. There are still a number of bugs and regressions that need to be fixed in the latest Vizia version before I can update to the latest version. See the vizia-update branch for the current status.

@everdrone
Copy link

@robbert-vdh was the vizia-update branch moved? I'm getting a 404

@geom3trik
Copy link
Contributor Author

@robbert-vdh was the vizia-update branch moved? I'm getting a 404

It was merged into main 🙂 but Robbert did it himself so this one is no longer valid.

@geom3trik geom3trik closed this Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants